new lint: suspicious_doc_comments #10497

y21 · 2023-03-13T12:28:34Z

This PR adds a new lint (suspicious_doc_comments) that triggers when the user writes ///! or /**!.
This is almost certainly a mistake and the user probably meant to write an inner doc comment (//!, /*!) to document the module or crate that this comment is contained in.

changelog: [suspicious_doc_comments]: new lint

rustbot · 2023-03-13T12:28:39Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dswij (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

bors · 2023-03-16T10:51:35Z

☔ The latest upstream changes (presumably #10481) made this pull request unmergeable. Please resolve the merge conflicts.

dswij

Thanks for the PR! and welcome to Clippy.

Some minor comments and suggestions.

Also, not sure if I personally love the name outer_doc_comment_bang, but combined with the allow attribute, it looks neat. I don't know if there is a better name for it though.

clippy_lints/src/outer_doc_comment_bang.rs

tests/ui/outer_doc_comment_bang.rs

y21

Also, not sure if I personally love the name outer_doc_comment_bang, but combined with the allow attribute, it looks neat. I don't know if there is a better name for it though.

I agree, I had a hard time coming up with a name for this lint. It seems like the linked issue that initially suggested this lint actually has a name suggestion (suspicious_doc_comment), totally missed that. I like that one. I'd be fine with renaming this to suspicious_doc_comment.

clippy_lints/src/outer_doc_comment_bang.rs

y21 · 2023-03-31T11:52:58Z

Oops, I didn't mean to start a new review there 😅. The comment I left was in response to changing it from "merging" the spans together to linting the lines individually.

y21 · 2023-04-01T12:18:11Z

I changed it to use mutlispan_sugg instead. Would suspicious_doc_comment be a better name for this lint or is this one fine?

dswij

Lint looks good! Just a couple small question/comment, and it'd be good to merge aftewards.

I think suspicious_doc_comments is nice because we have other similarly-named lints. Also note that the name would be plural because of Clippy's lint naming convention

clippy_lints/src/outer_doc_comment_bang.rs

Alexendoo · 2023-04-04T11:43:53Z

This error seems to be common which makes this a very useful lint

From the search, there's a few styles that would be good to add as tests

Both of these would be joined doc comments but in the first the intention would be to have two separate comments still

https://github.com/Twinklebear/tray_rust/blob/9359cb1868a723431214c9fa1fc8c1aef54adae3/src/sampler/morton.rs#L1-L4
https://github.com/openebs/mayastor/blob/aebfd876be16fa2676f311c36276efd6ad3aa019/sysfs/src/lib.rs#L1-L2

Various gaps

https://github.com/massalabs/massa/blob/b27e8416efac19e8ec470e96bb49c74687092ac3/massa-async-pool/src/changes.rs#L14-L16
https://github.com/Keats/jsonwebtoken/blob/8fba79b25459eacc33a80e1ee37ff8eba64079ca/src/jwk.rs#L2-L6

////! is probably a commented out outer doc comment

https://github.com/tanakh/reqwest/blob/147831375613a5e508487b2d85a99104ae1505af/src/lib.rs#L184-L185

It would also be good to see if it works on items that don't generally have doc comments applied to them, like

///! a
use x;

///! b
macro_rules! x {() => {}}

You don't have to explicitly handle all of these, particularly the first two are kind of conflicting, but it'd be good to have tests for them anyway

y21 · 2023-04-04T13:39:46Z

https://github.com/Keats/jsonwebtoken/blob/8fba79b25459eacc33a80e1ee37ff8eba64079ca/src/jwk.rs#L2-L6

This is one of the "known-but-deliberately-ignored" problems, I think, so I don't think I can add a test for this, can I? // run-rustfix would make this fail the test

Alexendoo · 2023-04-04T14:11:01Z

You can put them in a separate suspicious_doc_comments_unfixable.rs without // run-rustfix

y21 · 2023-04-05T14:28:21Z

Re the label: is there something left to do? I think I have a test for all the cases that were mentioned

dswij · 2023-04-06T03:45:54Z

@y21 oh, there's a conflict, so I thought you are still working on it :)

y21 · 2023-04-06T20:16:50Z

Fixed the conflicts and cleaned up the commit history a bit.

bors · 2023-04-07T07:54:27Z

☔ The latest upstream changes (presumably #10601) made this pull request unmergeable. Please resolve the merge conflicts.

dswij · 2023-04-07T11:14:05Z

@y21 Huge thanks for this! Should be a really nice, useful lint

@bors r+

bors · 2023-04-07T11:14:06Z

📌 Commit 5d01e6e has been approved by dswij

It is now in the queue for this repository.

bors · 2023-04-07T11:14:13Z

⌛ Testing commit 5d01e6e with merge 4a2cb5a...

bors · 2023-04-07T11:26:29Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 4a2cb5a to master...

rustbot assigned dswij Mar 13, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 13, 2023

dswij requested changes Mar 31, 2023

View reviewed changes

y21 commented Mar 31, 2023

View reviewed changes

clippy_lints/src/outer_doc_comment_bang.rs Outdated Show resolved Hide resolved

dswij reviewed Apr 3, 2023

View reviewed changes

clippy_lints/src/outer_doc_comment_bang.rs Outdated Show resolved Hide resolved

clippy_lints/src/outer_doc_comment_bang.rs Outdated Show resolved Hide resolved

y21 changed the title ~~new lint: outer_doc_comment_bang~~ new lint: suspicious_doc_comments Apr 4, 2023

dswij added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 5, 2023

y21 force-pushed the outer_doc_comment_bang branch 2 times, most recently from 8966407 to 32e9d4b Compare April 6, 2023 20:06

new lint: suspicious_doc_comments

5d01e6e

y21 force-pushed the outer_doc_comment_bang branch from 32e9d4b to 5d01e6e Compare April 7, 2023 10:05

dswij added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Apr 7, 2023

dswij self-requested a review April 7, 2023 11:08

dswij approved these changes Apr 7, 2023

View reviewed changes

bors merged commit 4a2cb5a into rust-lang:master Apr 7, 2023

bors mentioned this pull request Apr 7, 2023

Add items_after_test_module lint #10578

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new lint: suspicious_doc_comments #10497

new lint: suspicious_doc_comments #10497

y21 commented Mar 13, 2023 •

edited

Loading

rustbot commented Mar 13, 2023

bors commented Mar 16, 2023

dswij left a comment

y21 left a comment

y21 commented Mar 31, 2023 •

edited

Loading

y21 commented Apr 1, 2023

dswij left a comment

Alexendoo commented Apr 4, 2023

y21 commented Apr 4, 2023

Alexendoo commented Apr 4, 2023

y21 commented Apr 5, 2023

dswij commented Apr 6, 2023

y21 commented Apr 6, 2023

bors commented Apr 7, 2023

dswij commented Apr 7, 2023

bors commented Apr 7, 2023

bors commented Apr 7, 2023

bors commented Apr 7, 2023

new lint: suspicious_doc_comments #10497

new lint: suspicious_doc_comments #10497

Conversation

y21 commented Mar 13, 2023 • edited Loading

rustbot commented Mar 13, 2023

bors commented Mar 16, 2023

dswij left a comment

Choose a reason for hiding this comment

y21 left a comment

Choose a reason for hiding this comment

y21 commented Mar 31, 2023 • edited Loading

y21 commented Apr 1, 2023

dswij left a comment

Choose a reason for hiding this comment

Alexendoo commented Apr 4, 2023

y21 commented Apr 4, 2023

Alexendoo commented Apr 4, 2023

y21 commented Apr 5, 2023

dswij commented Apr 6, 2023

y21 commented Apr 6, 2023

bors commented Apr 7, 2023

dswij commented Apr 7, 2023

bors commented Apr 7, 2023

bors commented Apr 7, 2023

bors commented Apr 7, 2023

y21 commented Mar 13, 2023 •

edited

Loading

y21 commented Mar 31, 2023 •

edited

Loading